libbpf-cargo: Relative anonymous type naming for struct fields #1178
Conversation
|
Haven't looked at it yet, but this is great. I was considering doing that for |
Previously, anonymous types were named as __anon_{id} with id as a global counter. This approach caused cascading renaming of all subsequent anonymous definitions after any C anonymous struct order/amount change. This was a major design problem not for only development but even portability, since here may be the case when something was changed in vmlinux.h leading to compilation errors.
This commit addresses the issue by localizing cascading effects to the local struct layer rather than the global layer. While the solution is not fully ideal - cascading still occurs within individual structs - it eliminates the need for wide Rust code modifications when the order of anonymous struct definitions changes.
Note, that the commit does not fully eliminates __anon_{id}, but the most important part of it.
The commit is breaking and should be included in major version update.
|
Here are still some plain __anon types like in |
libbpf-cargo/src/gen/btf.rs
Outdated
| } | ||
|
|
||
| impl TypeMap { | ||
| pub fn derive_parent<'s>(&self, ty: &BtfType<'s>, parent: &BtfType<'s>) { |
There was a problem hiding this comment.
| pub fn derive_parent<'s>(&self, ty: &BtfType<'s>, parent: &BtfType<'s>) { | |
| fn derive_parent<'s>(&self, ty: &BtfType<'s>, parent: &BtfType<'s>) { |
There was a problem hiding this comment.
Is the name really fitting? Aren't you more registering a parent relationship rather than "deriving" anything?
There was a problem hiding this comment.
Perhaps register_parent would fit?
What do you mean exactly? Where is "here"? |
|
In 0e504e6 I simplified the code slightly, modified the global anon counting behavior, added documentation for new structure fields and included a test for depth-aligned anonymous structs. The global anonymous struct counting now excludes relative anons to make its behavior more representative and easier to use. I also added some fallback protection on it for (im)possible case when global anonymous struct is assigned before a relative one: in such case we will use old type of naming.
Nvm. Those anonymous structs are at the global level and should be kept with old naming. I think the code is ok now, but I'm aware of possible name collisions for relative types. |
|
Thanks, it looks better now!
Can you provide an example of such a case? |
I'm almost sure here is no the case. I was just thinking about it |
Add a CHANGELOG entry for pull request libbpf#1178, which adjusted the naming of generated Rust types for anonymous C equivalents. Signed-off-by: Daniel Müller <[email protected]>
Add a CHANGELOG entry for pull request #1178, which adjusted the naming of generated Rust types for anonymous C equivalents. Signed-off-by: Daniel Müller <[email protected]>
Previously, anonymous types were named as _anon{id} with id as a global counter. This approach caused cascading renaming of all subsequent anonymous definitions after any C anonymous struct order/amount change. This was a major design problem not for only development but even portability, since here may be the case when something was changed in vmlinux.h leading to compilation errors.
This PR addresses the issue by localizing cascading effects to the local struct layer rather than the global layer. While the solution is not fully ideal - cascading still occurs within individual structs - it eliminates the need for wide Rust code modifications when the order of anonymous struct definitions changes.
The PR is breaking and should be included in major version update.
The PR fixes #1161
While this approach is ok, I still think the better way will be to use getters, since this is breaking and less intuitive.